-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create the RequestCallPage #3697
Conversation
…/Expensify.cash into jasper-requestCallPageNavSetup
…port data that didn't exist
@Dal-Papa Just took this off hold, so we should be good for a review. Thanks!! |
src/pages/RequestCallPage.js
Outdated
...withLocalizePropTypes, | ||
|
||
/** The personal details of the person who is logged in */ | ||
myPersonalDetails: PropTypes.shape({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*/ | ||
getPhoneNumber(loginList) { | ||
const secondaryLogin = _.find(loginList, login => Str.isSMSLogin(login.partnerUserID)); | ||
return secondaryLogin ? Str.removeSMSDomain(secondaryLogin.partnerUserID) : null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe something to consolidate with Joe's code ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Dal-Papa Joe's code assumes that the user is an SMS user, so it isn't entirely possible to reuse what he has. He assumes that the user's phone number is located somewhere in their personalDetails object, which isn't always the case for this particular page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Re-arranging your screencasts just to label them and then merging.
Thanks!! I figured they were already labeled in the title, but probably a good idea. |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging in version: 1.0.73-4🚀
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import styles from '../styles/styles'; | ||
import Text from './Text'; | ||
import themeColors from '../styles/themes/default'; | ||
import withLocalize, {withLocalizePropTypes} from './withLocalize'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you use JSX then React
must be included
@@ -0,0 +1,171 @@ | |||
import {Component} from 'react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
React
must be included
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @marcaaron, sorry for the late reply! I noticed that this was broken and pushed a fix. It seems like it was broken when I merged a remote and local version of my branch.
Sorry was on an older branch actually seems like maybe it was fixed. |
👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open
|
🚀 Deployed to production in version: 1.0.74-0🚀
|
Details
This PR creates the RequestCallPage and sets it up with react-navigation so that it can be accessed via navigating to
<baseURL>/request-call
. It also modifies the onPress handler for the call button in the Concierge DM to pull up the RequestCallPage.Fixed Issues
Fixes https://github.com/Expensify/Expensify/issues/167901
Fixes https://github.com/Expensify/Expensify/issues/167902
Tests
<baseURL>/request-call
. Verify that the placeholder component displays.Expensiworks/InboxCallUser
job is queued up in Bedrock.QA Steps
<baseURL>/request-call
. Verify that the placeholder component displays.Expensiworks/InboxCallUser
job is queued up in BJM.Tested On
Screenshots
iOS
ios.mp4
Web
web.mp4
Desktop
desktop.mp4